Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: apache license eslint rule #27

Merged
merged 3 commits into from
Feb 1, 2024

Conversation

TannerGilbert
Copy link
Contributor

@TannerGilbert TannerGilbert commented Jan 26, 2024

Adds Eslint plugin to check if files contain license header.

Used plugin: https://github.com/Stuk/eslint-plugin-header

Removed all the Eslint files for the packages as they are currently not customizing anything. If a Eslint file is added to a package the header rule must be overridden to change the path of the file:

Example:

module.exports = require('@backstage/cli/config/eslint-factory')(__dirname, {
  overrides: [
    {
      "files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
      "rules": {
        "header/header": ["error", "../../header.js"]
      }
    }
  ],
});

Known issues:

.eslintrc.cjs Outdated
'\t distributed under the License is distributed on an "AS IS" BASIS,',
'\t WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.',
'\t See the License for the specific language governing permissions and',
'\t limitations under the License.',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should get rid of the tabs, looks weirdly formatted

There is also a short version available https://www.apache.org/foundation/license-faq.html#Apply-My-Software

Copyright [yyyy] [name of copyright owner]
SPDX-License-Identifier: Apache-2.0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to verify if the shortened version of the license header can be applied. I think @agrimmer or OSPO might have a better idea in this case. For now, I would prefer it to be more verbose than possibly needed - better safe than sorry :)

But yeah, if the tabs can be eliminated, that'd be great. I think (but I am not 100% sure) that template strings might be able to handle this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I woud keep the long version as seems to be standard for other plugins. See e.g.
https://github.com/backstage/backstage/blob/master/plugins/github-issues/src/index.ts

However, can we remove the brackets in [2024] and [Dynatrace].

@MrManny MrManny requested a review from agrimmer January 29, 2024 10:13
Copy link
Collaborator

@MrManny MrManny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change requests:

  • adjust pattern and template to avoid square brackets,
  • if possible, the tabs in the rest of the rule could be improved as per previous comments.

.eslintrc.cjs Outdated Show resolved Hide resolved
Signed-off-by: gilbert.tanner <[email protected]>
@MrManny MrManny self-requested a review February 1, 2024 15:07
@TannerGilbert TannerGilbert merged commit 82bc462 into main Feb 1, 2024
1 check passed
@johannes-b johannes-b added this to the 1.0 (GA) milestone Apr 3, 2024
@johannes-b johannes-b deleted the chore/apache-license-eslint-rule branch August 29, 2024 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants